Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[dagster-airlift] Fix materialization reporting order for tasks #23326

Merged
merged 3 commits into from
Aug 3, 2024

Conversation

dpeng817
Copy link
Contributor

@dpeng817 dpeng817 commented Jul 29, 2024

Fixes the materialization reporting order for asset keys mapped from airflow tasks to instead be topological.
Added a very basic test where a dependency between two tasks should always yield the downstream task being added after the upstream.

@schrockn
Copy link
Member

Consider toposort_flatten

@dpeng817 dpeng817 force-pushed the dpeng817/fix_materialization_reporting_order branch 2 times, most recently from 421e541 to b853b76 Compare July 29, 2024 23:46
@dpeng817 dpeng817 force-pushed the dpeng817/fix_tests branch from eec0f12 to 6a7ab7b Compare July 30, 2024 00:38
@dpeng817 dpeng817 force-pushed the dpeng817/fix_materialization_reporting_order branch from b853b76 to b7f87ae Compare July 30, 2024 00:38
@dpeng817 dpeng817 force-pushed the dpeng817/fix_materialization_reporting_order branch from b7f87ae to 259d879 Compare July 31, 2024 04:21
@dpeng817 dpeng817 changed the base branch from dpeng817/fix_tests to master July 31, 2024 04:21
@dpeng817 dpeng817 force-pushed the dpeng817/fix_materialization_reporting_order branch from 259d879 to f3527fb Compare July 31, 2024 04:22
@dpeng817 dpeng817 changed the base branch from master to dpeng817/fix_tests July 31, 2024 04:22
Copy link

Deploy preview for dagit-storybook ready!

✅ Preview
https://dagit-storybook-c6ns0gxtl-elementl.vercel.app
https://dpeng817-fix-materialization-reporting-order.components-storybook.dagster-docs.io

Built with commit 259d879.
This pull request is being automatically deployed with vercel-action

Copy link

Deploy preview for dagit-core-storybook ready!

✅ Preview
https://dagit-core-storybook-pqmj8txfd-elementl.vercel.app
https://dpeng817-fix-materialization-reporting-order.core-storybook.dagster-docs.io

Built with commit 259d879.
This pull request is being automatically deployed with vercel-action

@dpeng817 dpeng817 requested a review from schrockn July 31, 2024 05:31
@dpeng817 dpeng817 changed the title Fix materialization reporting order for tasks [dagster-airlift] Fix materialization reporting order for tasks Jul 31, 2024
@@ -217,9 +218,12 @@ def airflow_dag_sensor(context: SensorEvaluationContext) -> SensorResult:
"Link to Run": MarkdownMetadataValue(
f"[View Run]({airflow_webserver_url}/dags/{dag_id}/grid?dag_run_id={dag_run['dag_run_id']}&tab=details)"
),
"Creation Timestamp": TimestampMetadataValue(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Consider "Event Creation Timestamp"

Base automatically changed from dpeng817/fix_tests to master August 3, 2024 23:52
@dpeng817 dpeng817 merged commit 430e24f into master Aug 3, 2024
1 check passed
@dpeng817 dpeng817 deleted the dpeng817/fix_materialization_reporting_order branch August 3, 2024 23:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants